Fix a latent SMEM handoff race in the SM100 DSA indexer backward kernel#426
Fix a latent SMEM handoff race in the SM100 DSA indexer backward kernel#426zkyue wants to merge 2 commits into
Conversation
In _load_warp all 32 lanes of the load warp store sW / sGradSignal to SMEM, but only an elected lane arrives on MBAR_W_LOADED. Per the PTX memory model, mbarrier.arrive (release, cta scope) orders only the executing thread's prior accesses, so the other 31 lanes' stores have no happens-before edge to the compute warpgroup's mbarrier_wait and subsequent reads: a formal data race. Latent in practice: no corruption observed on the tested B200 / CUDA 13.3 / cutlass-dsl 4.6.1 build, whose captured SASS (topk=128 specialization) carries an unpredicated MEMBAR.ALL.CTA before the arrive; that compensation is not contractual. Fix: initialize MBAR_W_LOADED with count WARP_SIZE and have all 32 lanes arrive, closing the happens-before chain per lane. W_LOADED is a one-shot handoff (single arrive site, single phase-0 wait), so the count change is self-contained. Verified: compute-sanitizer racecheck hazards on kernel_gemm drop to 0 across 1-CTA, 512-CTA and batch=3/topk=512 shapes (previously the only flagged site in those runs); d_index_q / d_weights byte-identical to unpatched on the tested shapes; upstream DSA pytest results unchanged. Signed-off-by: zky <51477259+zkyue@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe SM100 indexer backward kernel changes ChangesSM100 barrier synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@python/cudnn/deepseek_sparse_attention/indexer_backward/indexer_backward_sm100.py`:
- Around line 544-546: Update the barrier initialization block containing
mbarrier_init for MBAR_W_LOADED and MBAR_DQ_DONE to execute inside
cute.arch.elect_one(), ensuring only one lane performs setup. Keep
cute.arch.sync_threads() after the guarded block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e3a74215-9c58-4800-b62a-a1f2551448d7
📒 Files selected for processing (1)
python/cudnn/deepseek_sparse_attention/indexer_backward/indexer_backward_sm100.py
Review follow-up: the barrier-initialization block under `if warp_idx == 0:` was executed by all 32 lanes of warp 0, i.e. each mbarrier_init ran 32 times on the same SMEM barrier object. Redundant re-initialization before the sync_threads is benign on current hardware, but a single initializing thread is the contract the PTX ISA documents for mbarrier.init, and every other kernel in this package already wraps barrier init in an election. Wrap the block in `with cute.arch.elect_one():` so exactly one lane performs the init; the trailing sync_threads() ordering is unchanged. No functional change intended or observed. Re-verified on B200 / CUDA 13.3 / cutlass-dsl 4.6.1: compute-sanitizer racecheck 0 hazards on the 1-CTA (hazard-level report), 512-CTA and batch=3/topk=512 shapes; fe_api/dsa pytest results identical to the parent commit (same 26 passed / 4 skipped / same 4 environment-specific failures); 30-replay d_index_q / d_weights SHA-256 byte-identical to the parent commit. Signed-off-by: zky <51477259+zkyue@users.noreply.github.com>
Before submitting
pre-commit runand committed any formatting changes. (clang-format n/a — Python only; black 26.3.1-l 160passes, file unchanged.)Affected area
FE OSS kernels or CuTeDSL
Summary
compute-sanitizer --tool racecheckflags RAW hazards onkernel_gemmofindexer_backward_sm100.py(IndexerBackwardSm100). Every flagged hazard traces to onedefect in
_load_warp: all 32 lanes of the load warp storesW/sGradSignalto SMEM,but only a single elected lane arrives on
MBAR_W_LOADED:The compute warpgroup (warps 4–7) does
mbarrier_wait(MBAR_W_LOADED)and then readssW/sGradSignalwith plain shared loads. Per the PTX memory model,mbarrier.arrive(default
.release,.ctascope) releases only the prior memory accesses of theexecuting thread, so the stores of the other 31 lanes have no happens-before edge to
the consumer's reads. This is a formal data race.
Fix (2 logical lines): initialize
MBAR_W_LOADEDwith countWARP_SIZEand have all 32lanes of the load warp arrive. Each lane's release-arrive then orders its own stores
before the consumer's acquire-wait.
MBAR_W_LOADEDis a one-shot handoff (single arrivesite, single phase-0 wait), so the count change is self-contained.
Why
Evidence that this is a real memory-model violation, not a sanitizer false positive:
--racecheck-report hazard --print-limit 0reports exactly 620 byte-level hazards:the recorded writer is always warp-0 lanes 1–31, and the same store instructions
executed by lane 0 are never flagged. 620 = 4
sGradSignalstores × 31 lanes × 4 B +2
sWstores × 31 lanes × 2 B, byte-exact. Racecheck therefore models the mbarrierarrive→wait edge and applies it precisely to the arriving thread — matching the PTX
model, and pinpointing exactly the 31 lanes whose stores are unordered.
kernel_gemm— whole-grouparrivals (DS_READY / DK_EMPTY / K_LOADED, count = 128) and single-signal
tcgen05.commitbarriers (S_FULL / DK_FULL / K_CONSUMED / DQ_DONE) — report zerohazards in the same runs, including at a multi-block shape (b=3, s_q=64, s_kv=2048, topk=512) that
exercises the full 3-stage sK / 2-stage TMEM pipeline.
W_LOADEDwas the onlysingle-elected arrive standing in for whole-warp generic SMEM stores.
nvcc -arch=sm_100a):= 31 lanes × 4 B, lane 0 exempt — the same signature as the real kernel.
__syncwarp()then lane-0-only arrive): still 124. This variant is arguablycorrect under the CUDA C++
__syncwarpmemory-ordering guarantee, but racecheck doesnot chain the syncwarp edge into the mbarrier release; the all-lane arrive is the
variant whose ordering is explicit in the PTX memory model and sanitizer-clean, so
that is what this PR uses.
Why latent. We did not observe corruption on the build we tested (B200, CUDA 13.3,
nvidia-cutlass-dsl 4.6.1):
d_index_q/d_weightsare bitwise stable across 30 replayseven unpatched, and in the SASS we captured for the topk=128 kernel specialization, ptxas lowers the
release-arrive as an unpredicated
MEMBAR.ALL.CTAexecuted by all lanes, which restoresthe ordering as a side effect. That compensation is not contractual — an nvcc 13.3 compile of the same
pattern (calibration V0) contains no
MEMBARbefore the arrive — so the pattern shouldnot be relied on across compiler versions. Fixing it also makes
kernel_gemmracecheck-clean, so future real regressions are not buried in baseline noise.
Related issues
None filed; found by running compute-sanitizer over the DSA OSS kernels.
API and compatibility impact
No API change.
d_index_q/d_weightsare byte-identical between unpatchedand patched builds (direct byte comparison of outputs; full-sha256 stable 30/30 replays
per side at b=3, s_q=64, s_kv=2048, topk=512).
d_index_kis accumulated with fp32atomics and is run-to-run nondeterministic on both sides (maximum relative deviation from the first replay, over 30
replays: 4.0e-6 unpatched vs 4.1e-6 patched).
processes, 16 pairs per shape, 200 profiled launches per run,
cuda_gpu_kern_sum):kernel_gemm−0.014% ± 0.017% (Student-t 95% CIon paired deltas) — no measurable change.
kernel_gemm+0.218% ± 0.016% (~+74 ns on a34.1 µs launch) — a small cost on this shape. It reproduces with the run
order swapped (+0.204% ± 0.017%, 8 pairs), while the untouched
ScoreGradkernelmeasured in the same processes stays flat (|Δ| ≤ 0.03%), consistent with a small
systematic cost rather than measurement noise. Mechanically the patch adds 31 extra
single-thread arrives on one mbarrier, once per CTA, on the load warp.
Testing
Environment: B200 (SM100), CUDA 13.3, compute-sanitizer 2026.2.0.0, Nsight Systems
2026.3.1, nvidia-cutlass-dsl 4.6.1, torch 2.13, Python package built from this branch
(1.27.0).
racecheck (
compute-sanitizer --tool racecheck, kernel driven throughindexer_backward_wrapper; "reports" = default analysis-mode error reports, whichaggregate the underlying per-byte hazards):
--racecheck-report hazard --print-limit 0All unpatched reports point at the
sW/sGradSignalstores in_load_warp; no othersite in the kernel is flagged before or after the patch.
pytest (
test/python$ pytest fe_api/dsa/ -q, default L0): identical results onunpatched base and this branch — 26 passed, 4 skipped, 4 failed, with the same 4 failing
cases on both sides (
test_DSA_indexer_top_k×2,test_DSA_sparse_attention_backward×2— reference-comparison failures present on unpatched develop@3a9ed3f in our environment,
untouched by this change). All
test_DSA_indexer_backwardcases pass on both sides.Numerics and performance: as under "API and compatibility impact".
Calibration repro (mbar_calib.cu) — isolates what racecheck models
Summary by CodeRabbit